-
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
shinyvalidate
improvements
#199
Conversation
Concerning gather_fails: Like the idea and the example app. My comments / questions in the conversations. |
All modules have been modified. |
I add |
Sorry, maybe I shouldn't worry about the misspelling but I thought I just pointed out so we don't forget about it. Overall, I think collating the validation messages is a great idea and will improve user's experience! One scenario that I could recall recently is when a user got frustrated as he got into another issue after fixing one issue. I think you pointed out that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I decided to add debouncing to |
Code Coverage Summary
Diff against main
Results for commit: a47863c Minimum allowed coverage is ♻️ This comment has been updated with latest results |
One minor thing I noticed that is slightly (5%) annoying: uppercases and lowercases in labels. For example, in spiderplot, we have "Variable" used everywhere, while in patient profile it is "variable". It also goes for other packages. I noticed that developers usually just go with the convention used by labels when they create warning message, and it makes sense, but it could be managed on some lazy day. |
I was equally annoyed by this but assumed it was due to independent design decisions by different module developers. |
So the only problem is that the check() output is not full of |
We decided to ignore them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Closes this issue
Following the introduction of
validate_inputs
toteal
by #199,this PR:
validate
calls toshinyvalidate
input validatorsvalidate_input
funcitons